Remove unified flag usage, rely on host metadata#720
Remove unified flag usage, rely on host metadata#720hectorcast-db wants to merge 4 commits intomainfrom
Conversation
## 🥞 Stacked PR - [**#710 Add cloud field to HostMetadata**](#710) [[Files](https://github.com/databricks/databricks-sdk-java/pull/710/files)] - [#711 Fix GetWorkspaceClient for unified account hosts](#711) [[Files](https://github.com/databricks/databricks-sdk-java/pull/711/files)] - [#712 Add test for GetWorkspaceClient with SPOG host](#712) [[Files](https://github.com/databricks/databricks-sdk-java/pull/712/files)] - [#713 Call resolveHostMetadata on Config init](#713) [[Files](https://github.com/databricks/databricks-sdk-java/pull/713/files)] - [#714 Resolve TokenAudience from host metadata for account hosts](#714) [[Files](https://github.com/databricks/databricks-sdk-java/pull/714/files)] - [#718 Make GCP SA token refresh non-blocking](#718) [[Files](https://github.com/databricks/databricks-sdk-java/pull/718/files)] - [#719 Add integration test for host metadata resolution](#719) [[Files](https://github.com/databricks/databricks-sdk-java/pull/719/files)] - [#720 Remove unified flag usage, rely on host metadata](#720) [[Files](https://github.com/databricks/databricks-sdk-java/pull/720/files)] --------- ## Summary Port of Go SDK [#1512](databricks/databricks-sdk-go#1512). Adds a `cloud` field to `HostMetadata` that is populated from the `/.well-known/databricks-config` discovery endpoint. **Why:** Today, `isAws()`, `isAzure()`, and `isGcp()` infer cloud type by suffix-matching the workspace hostname against a hardcoded list of known DNS zones. This works for standard deployments but fails for non-standard hostnames (custom vanity domains, unified hosts, etc.). The discovery endpoint is the authoritative source and already returns a `cloud` field, but the SDK was discarding it. **Changes:** - `HostMetadata`: new `cloud` field (`@JsonProperty("cloud")`), getter, and 4-arg constructor - `HostMetadataTest`: deserialization with/without cloud, constructor tests `NO_CHANGELOG=true` ## Test plan - [x] `HostMetadataTest`: 4 tests for cloud field deserialization and constructors
78816e5 to
483ae61
Compare
eebd798 to
4607961
Compare
## 🥞 Stacked PR - [#710 Add cloud field to HostMetadata](#710) [[Files](https://github.com/databricks/databricks-sdk-java/pull/710/files)] - [**#711 Fix GetWorkspaceClient for unified account hosts**](#711) [[Files](https://github.com/databricks/databricks-sdk-java/pull/711/files)] - [#712 Add test for GetWorkspaceClient with SPOG host](#712) [[Files](https://github.com/databricks/databricks-sdk-java/pull/712/files)] - [#713 Call resolveHostMetadata on Config init](#713) [[Files](https://github.com/databricks/databricks-sdk-java/pull/713/files)] - [#714 Resolve TokenAudience from host metadata for account hosts](#714) [[Files](https://github.com/databricks/databricks-sdk-java/pull/714/files)] - [#718 Make GCP SA token refresh non-blocking](#718) [[Files](https://github.com/databricks/databricks-sdk-java/pull/718/files)] - [#719 Add integration test for host metadata resolution](#719) [[Files](https://github.com/databricks/databricks-sdk-java/pull/719/files)] - [#720 Remove unified flag usage, rely on host metadata](#720) [[Files](https://github.com/databricks/databricks-sdk-java/pull/720/files)] --------- ## Summary Port of Go SDK [#1517](databricks/databricks-sdk-go#1517). Fixes `getWorkspaceClient()` for unified account hosts that don't follow the standard environment DNS zone pattern (e.g. SPOG/unified hosts). Previously, the workspace host was always constructed via `getDeploymentUrl(ws.getDeploymentName())`, which blindly appends the environment's DNS zone. For unified hosts where the account and workspace share the same host, this produces an incorrect URL. **Changes:** - `AccountClient.getWorkspaceClient()`: clones config instead of mutating `this.config` for unified hosts **Note:** `AccountClient.java` is a generated file. The template needs to be updated. `NO_CHANGELOG=true` ## Test plan - [x] `AccountClientTest`: existing tests pass
483ae61 to
ad7024c
Compare
4607961 to
153a890
Compare
ad7024c to
b87fca5
Compare
153a890 to
821337f
Compare
821337f to
f79a3e8
Compare
72fae6f to
34394bc
Compare
## 🥞 Stacked PR Use this [link](https://github.com/databricks/databricks-sdk-java/pull/719/files) to review incremental changes. - [**hectorcast-db/stack/port-7-integration-test-metadata**](#719) [[Files changed](https://github.com/databricks/databricks-sdk-java/pull/719/files)] - [hectorcast-db/stack/port-8-remove-unified-flag](#720) [[Files changed](https://github.com/databricks/databricks-sdk-java/pull/720/files/cbd81a9a8534a735250b64fe3957a3923b6da899..34394bc5a5eb92745574c42c336a326c8719cebd)] --------- ## Summary Port of Go SDK [#1546](databricks/databricks-sdk-go#1546). Adds an integration test verifying that `config.resolve()` populates `accountId`, `workspaceId`, and `discoveryUrl` from the host's `/.well-known/databricks-config` endpoint. **Changes:** - `HostMetadataIT`: integration test for metadata resolution (requires `DATABRICKS_HOST` via `ucws` env context) `NO_CHANGELOG=true` ## Test plan - [x] All unit tests pass (no production code changes) - [x] `HostMetadataIT` runs against live workspace
Port of Go SDK #1547. Removes HostType.UNIFIED and all runtime checks of experimentalIsUnifiedHost. Host type is now determined solely from URL pattern (accounts.* = ACCOUNTS, else WORKSPACE). Host metadata resolution from /.well-known/databricks-config (added in PR 4) handles populating accountId, workspaceId, and discoveryUrl automatically. Key changes: - getHostType(): no longer returns UNIFIED - isAccountClient(): no longer throws for unified hosts - getClientType(): simplified, no UNIFIED case - fetchDefaultOidcEndpoints(): removed unified OIDC branch - DatabricksCliCredentialsProvider: removed --experimental-is-unified-host - AccountClient.getWorkspaceClient(): uses DNS zone matching (like Go SDK) to decide whether to reuse host or build deployment URL Co-authored-by: Isaac
34394bc to
849be46
Compare
Remove the experimentalIsUnifiedHost gate in tryResolveHostMetadata() so metadata is resolved unconditionally during config init. Add auto-stub for /.well-known/databricks-config in FixtureServer to prevent metadata resolution from interfering with unrelated tests. Update tests to remove redundant explicit resolveHostMetadata() calls and unnecessary setExperimentalIsUnifiedHost(true). Co-authored-by: Isaac
After removing the unified-host guard, all configs now resolve /.well-known/databricks-config. Update HttpPathTest and IdempotencyTestingAPITest mocks to account for this extra call. Co-authored-by: Isaac
Range-diff: main (884e9dd -> 27eafd3)
Reproduce locally: |
Range-diff: main (27eafd3 -> 7810145)
Reproduce locally: |
The openIDConnectEndPointsTestAccounts test was making real HTTP calls to accounts.cloud.databricks.com, causing failures when the endpoint returned HTML instead of JSON. Use a mock HttpClient that stubs the host metadata call, since account OIDC endpoints are computed locally. Also apply spotless formatting fixes. Co-authored-by: Isaac
7810145 to
19aa378
Compare
Range-diff: main (7810145 -> 19aa378)
Reproduce locally: |
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
tejaskochar-db
left a comment
There was a problem hiding this comment.
LGTM mod comment.
| /** | ||
| * Flag to explicitly mark a host as a unified host. Note: This API is experimental and may change | ||
| * or be removed in future releases without notice. | ||
| */ | ||
| @ConfigAttribute(env = "DATABRICKS_EXPERIMENTAL_IS_UNIFIED_HOST") | ||
| private Boolean experimentalIsUnifiedHost; |
There was a problem hiding this comment.
I'm guessing these can't be removed as it will break users that had set it? Maybe should still document that it is no longer used here?
🥞 Stacked PR
Use this link to review incremental changes.
Summary
Port of Go SDK #1547.
Removes
HostType.UNIFIEDand all runtime checks ofexperimentalIsUnifiedHost. Host type is now determined solely from URL pattern (accounts.* = ACCOUNTS, else WORKSPACE). Host metadata resolution (from earlier PRs in this stack) handles populating config fields automatically.Key changes:
getHostType(): no longer returnsUNIFIED; determined solely by URL patternisAccountClient(): no longer throws for unified hostsgetClientType(): simplified, noUNIFIEDcasefetchDefaultOidcEndpoints(): removed unified OIDC branch, removedgetUnifiedOidcEndpoints()DatabricksCliCredentialsProvider.buildHostArgs(): removed--experimental-is-unified-host,--account-id,--workspace-idflags for unified caseAccountClient.getWorkspaceClient(): uses DNS zone matching (like Go SDK) to decide whether to reuse host or build deployment URLHostTypeenum: removedUNIFIEDvalueauthenticate(): removedX-Databricks-Org-Idheader injection (handled by generated*Impl.javafiles)Note:
AccountClient.javais a generated file. The template needs to be updated.NO_CHANGELOG=trueTest plan
UnifiedHostTest,DatabricksConfigTest,AccountClientTest,DatabricksCliCredentialsProviderTestupdated